-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
MDEV-38499: cmake and compile errors on MacOSX when compiling mariadb from a git tree #4522
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
21b2b2c to
96b27db
Compare
96b27db to
81f8b6f
Compare
dr-m
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a particular reason why we are not working around the Bison bug in the earliest maintained branch (10.6)?
4b02673 to
9ea7208
Compare
|
I am sad to see that my experimental change 615340f in #3616 to |
compiling mariadb from a git tree Fixed sprintf deprecation warnings compiling on MacOSX. Replaced some sprintf calls with equivalent snprintf calls, enough so that "normal" compile on MacOSX (as documented in the docs) completes without warnings.
9ea7208 to
0417b45
Compare
vaintroub
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some formatting and comment nitpicks
| if (!srv_read_only_mode) { | ||
| if (srv_innodb_status) { | ||
|
|
||
| const size_t len = strlen(fil_path_to_mysql_datadir) + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Slightly weird indentation here, inconsistent with existing code. Can you align with what was here before (spaces? tabs? mix ? ), so it looks good in the github online page, too?
| (IF_WIN(GetCurrentProcessId(), getpid()))); | ||
| ut_malloc_nokey(len)); | ||
|
|
||
| snprintf(srv_monitor_file_name, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a lot of whitespace changes here. I think one line would work well, if you change sprintf to snprintf, and add , len at the end of the same line. Or on a new line, if you want, with indentation consistent with other parameters. But can the rest of parameters remain intact?
| sprintf(to, "%08lx%08lx", hash_res[0], hash_res[1]); | ||
| /* | ||
| we assume that to has at least 17 bytes allocated: | ||
| 2 ulongs in hex + NUL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"2 hexadecimals numbers, 8 bytes each + NUL"
That they are ulong, is non-essential (and also unnecessary, better if they used a more portable type)
But that they are 8 byte long, is essential.
Fix the warnings issued by a normal MacOSX compile.
Problems:
On 1: replaced the relevant sprintf with snprintf. On 2: worked around by adding a compiler define with a different name
aliasing the character set variable used with a name that
won't trigger the bison warning
On 3: This is due to the fact that there's a circular dependecy between
mysys and dbug (among others). Turned the warning off by adding a macro
to suppress the duplicate library warning.
Applied the macro to all targets that have a dependency on
the mysys/dbug/strings libraries